Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added test for get_indirect_is_a() and update to EMMO 1.0.0-beta5 #679

Closed
wants to merge 4 commits into from

Conversation

jesper-friis
Copy link
Collaborator

Description

Added test for get_indirect_is_a() in PR #678.

This test is kept in a separate PR since it expects EMMO 1.0.0-beta5. The update to 1.0.0-beta5 implies some additional updates...

Type of change

  • Bug fix.
  • New feature.
  • Documentation update.
  • Test update.

Checklist

This checklist can be used as a help for the reviewer.

  • Is the code easy to read and understand?
  • Are comments for humans to read, not computers to disregard?
  • Does a new feature has an accompanying new test (in the CI or unit testing schemes)?
  • Has the documentation been updated as necessary?
  • Does this close the issue?
  • Is the change limited to the issue?
  • Are errors handled for all outcomes?
  • Does the new feature provide new restrictions on dependencies, and if so is this documented?

Comments

ontopy/patch.py Outdated
Comment on lines 102 to 107
[locstr('ChemicalElement', 'en')]
['ChemicalElement']
>>> emmo.Atom['altLabel'] = 'Element'
>>> emmo.Atom['altLabel'] = locstr('Atomo', 'it')
>>> emmo.Atom['altLabel']
[locstr('ChemicalElement', 'en'), 'Element', locstr('Atomo', 'it')]
['ChemicalElement', 'Element', 'Atomo']
"""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which version of owlready2 do you have? With recent versions of owlready2 the locstr is returned.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have Owlready2 v0.41.

"http://emmo-repo.github.io/versions/1.0.0-beta4/emmo.ttl"
"http://emmo-repo.github.io/versions/1.0.0-beta5/emmo.ttl"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not do this, but keep the latest stable version as standard.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine for me. Then we can just drop this PR and skip testing get_indirect_is_a()

)
elif base_iri == "emmo-inferred":
base_iri = (
"https://emmo-repo.github.io/versions/1.0.0-beta4/"
"https://emmo-repo.github.io/versions/1.0.0-beta5/"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not do this, but keep the latest stable version as standard.

"emmo-inferred.ttl"
)
elif base_iri == "emmo-development":
base_iri = (
"https://emmo-repo.github.io/versions/1.0.0-beta4/"
"https://emmo-repo.github.io/versions/1.0.0-beta5/"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good, then we use emmo-development in the test

Comment on lines 95 to 96
def test_get_indirect_is_a(emmo: "Ontology") -> None:
assert any(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def test_get_indirect_is_a(emmo: "Ontology") -> None:
assert any(
def test_get_indirect_is_a() -> None:
from ontopy import get_ontology
emmo = get_ontology('emmo-development').load()
assert any(

Copy link
Collaborator

@francescalb francescalb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest updating only the default 'emmo-develeopment' to beta5 and keep the stable noes to beta4. Then we can use the development version for this updated test only.

For the other tests and code that you have changed I have created issue #680

where I have copied in your changes with updates to beta5 so that these are not lost once the upgrade to new stable version of EMMO is done.

@jesper-friis
Copy link
Collaborator Author

I suggest updating only the default 'emmo-develeopment' to beta5 and keep the stable noes to beta4. Then we can use the development version for this updated test only.

For the other tests and code that you have changed I have created issue #680

where I have copied in your changes with updates to beta5 so that these are not lost once the upgrade to new stable version of EMMO is done.

Fine with me. The valuable part is in PR #678. This PR just tests it. Feel free to close it.

francescalb added a commit that referenced this pull request Jan 16, 2024
# Description
Update test_unit_dimension in emmocheck to EMMO 1.0.0-beta5.

A test for this PR is added in PR #679.

## Type of change
<!-- Put an `x` in the box that applies. -->
- [x] Bug fix.
- [ ] New feature.
- [ ] Documentation update.
- [ ] Test update.

## Checklist
<!-- Put an `x` in the boxes that apply. You can also fill these out
after creating the PR. -->

This checklist can be used as a help for the reviewer.

- [ ] Is the code easy to read and understand?
- [ ] Are comments for humans to read, not computers to disregard?
- [ ] Does a new feature has an accompanying new test (in the CI or unit
testing schemes)?
- [ ] Has the documentation been updated as necessary?
- [ ] Does this close the issue?
- [ ] Is the change limited to the issue?
- [ ] Are errors handled for all outcomes?
- [ ] Does the new feature provide new restrictions on dependencies, and
if so is this documented?

## Comments
<!-- Additional comments here, including clarifications on checklist if
applicable. -->

---------

Co-authored-by: Francesca L. Bleken <[email protected]>
@francescalb
Copy link
Collaborator

Fixed in PR#678

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants